fix: close 1.0-readiness gaps (delete-guard, cross-target collisions, validation, docs) - #22
Merged
Merged
Conversation
…estroy files
Two live-reproduced data-loss bugs found during a 1.0-readiness review:
- buildDeleteGuard only protected the source-plugins root when
`source.plugins` was explicitly set in config; loadConfig applies the same
default ("plugins") regardless, so an unforced `prune`/`clean` could
silently delete real source files under the default, unconfigured path.
- assertNoCrossTargetCollisions only compared artifacts built in a single
`build()` invocation, so running `build --target X` after an earlier
`build --target Y` with an overlapping outDir would silently overwrite Y's
files, and a later `clean --target Y` would then delete what were now X's
live files.
Also makes build() write every target's new files before pruning any
target's stale ones, so a mid-build failure can't leave one target with its
stale files gone, new files partially written, and an inconsistent manifest.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cursor.validateManifest was a no-op; the real structural check only existed in the dev-only ajv schema test, so a user running `pluginpack validate --target cursor` got materially weaker protection than every other target's shipped validator. Adds a name-pattern check and unknown-top-level-key rejection, mirroring the vendored schema's required/additionalProperties rules, plus a regression test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Installs the real claude CLI in CI so `claude plugin validate --strict` (the strongest documented conformance oracle for the claude target) runs on every PR instead of silently skipping. - Adds Node 22.12.0 to the CI matrix alongside 24, so package.json's stated `engines` floor is actually exercised, not just declared. Fixes CLAUDE.md's contradictory "Node >= 24" line to match the real floor. - Adds scripts/verify-pack-contents.mjs and verify-packed-install.mjs: the former asserts the npm tarball's file list (not just that `npm pack --dry-run` exits zero); the latter installs the real tarball into a scratch project and runs the installed binary, catching a broken `bin` shebang or `files` misconfig no existing test could see. - Adds a `labels` CI job requiring one of RELEASE.md's changelog labels on every PR. - Removes the now-redundant `undici` override (release-it now declares it directly); keeps the `esbuild` override, which is still load-bearing (GHSA-g7r4-m6w7-qqqr requires >=0.28.1, outside tsup's own declared range). - Adds @vitest/coverage-v8 and a `test:coverage` script (no threshold gate). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…I caveats
- README badge: Experimental -> Prerelease. Shipping a 1.0.0 tag under an
"Experimental" badge (Glean's own stability doc: "not recommended for
production use") directly contradicted the tag; Prerelease ("feature-
complete, being validated before GA") matches where this actually stands.
- skills/add-pluginpack-target/SKILL.md described the pre-registry
architecture (src/targets.ts, src/validate.ts, ~5 touch points) that was
deleted in the target-registry migration; rewritten to match CLAUDE.md's
current architecture.
- Added OpenAI Codex CLI to the three skill docs and the README Quick Start
example that omitted it despite it being a fully supported 5th target.
- Documented in README's Programmatic API section: the target set is closed
(no third-party target-registration API), the package is deliberately
ESM-only, and Artifact/ResolvedProject's Map fields don't survive
JSON.stringify. Added the missing getInstallSnippetCitation/Citation
entries to the same table.
- Unexported SourceProvider from src/index.ts: nothing in build()/loadConfig()
accepts a source override yet, so the exported type committed to public
surface no one could actually use.
- Documented the deliberate choice not to use zod's .strict() on config
schemas, and what counts as a "breaking change" for a tool whose output is
files written into a consumer's repo (RELEASE.md).
- Re-verified the vendored Cursor schemas are still byte-identical to
upstream and recorded that check in SOURCE.md.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e, clean, docs Only `build` and `install-info` were ever exercised as the real built binary via bintastic; `validate`/`diff`/`prune`/`clean`/`init`/`docs` bypassed cli.ts's commander wiring entirely, so a broken option parse or wrong exit code in any of them had no test that could catch it. `init` in particular had zero coverage at any level, since it isn't a library function. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The labels job only ran on the default pull_request activity types (opened/synchronize/reopened), so a label attached moments after creation (e.g. via `gh pr create --label`, or by a maintainer during review) never retriggered the check — observed on this PR's own first run. Split it into its own workflow triggered on labeled/unlabeled too, and dropped it from ci.yml's default trigger so the main test matrix doesn't needlessly rerun on every label change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes found during a structured multi-agent readiness review ahead of a 1.0.0 tag. The two headline issues are live-reproduced data-loss bugs in the delete/build pipeline; everything else is process/coverage/docs hardening found alongside them.
prune/cleancould silently delete real source files.buildDeleteGuardonly protected the source-plugins root whensource.pluginswas explicitly set in config, butloadConfigapplies the same"plugins"default regardless — so an unforced prune under the default, unconfigured path deleted hand-written source with zero warning.--targetinvocations.assertNoCrossTargetCollisionsonly compared artifacts built in onebuild()call.build --target Xafter an earlierbuild --target Ywith an overlappingoutDirsilently overwrote Y's files, and a laterclean --target Ythen deleted what were now X's live files.build()now writes every target's new files before pruning any target's stale ones, so a mid-build failure can't leave stale-files-gone + new-files-partial + a stale manifest.cursor.validateManifestwas a no-op; it now does real field-level checks (name pattern + unknown-key rejection) matching the vendored schema, closing a gap where cursor's shippedvalidatewas weaker than every other target's.claudeCLI (soclaude plugin validate --strictactually runs instead of silently skipping), tests Node 22.12.0 alongside 24 (the statedenginesfloor was never exercised), asserts the packed npm tarball's actual file list, and installs+runs that tarball end-to-end. A newlabelsjob requires a changelog label on every PR.Mapfields underJSON.stringify, unusable exportedSourceProvidertype) were closed.undicioverride; keptesbuild's (still load-bearing — GHSA-g7r4-m6w7-qqqr needs >=0.28.1, outside tsup's own declared range).@vitest/coverage-v8(no threshold gate yet) and CLI-binary-level smoke tests forinit/validate/diff/prune/clean/docs, which previously bypassedcli.ts's commander wiring entirely in tests.Test plan
npm run checkpasses (format, lint, typecheck, 117 tests, build, docs sync)npm run audit— 0 production vulnerabilitiesnode scripts/verify-pack-contents.mjs— required files present in the tarballnode scripts/verify-packed-install.mjs— packed tarball installs and its CLI actually runs (--version,init)--targetbuild collision) and fail without the fix🤖 Generated with Claude Code